why do we need this error-handler/try-catch construction?
we don't need it in other rules/rule-helpers?

could we use @sscanf() instead and check the return type?

why do we need this error-handler/try-catch construction?

To support the runtime versions phpstan is running with (7.2 < 8.2 (dev) > 8.... IIRC)

we don't need it in other rules/rule-helpers?

I don't know ... maybe this is automatically downported by the easy downgrader?

could we use @sscanf() instead and check the return type?

I just had the same question/idea and wanted to ask you if that would be acceptable by code-style in phpstan.

For the technical question I'll explore this option and try to figure out a speaking snippet that runs on 3v4l.org if you like.

we have other places where we swallow runtime warnings with @ in case there is no other option.
(there should be a test-case in the PR, which fails when the @ gets removed)

The @-suppression operator looks like a technically feasible option for PHP 4.3.0 - 7.4.33 and the try-catch block is required for 8.0.0 - 8.5.3. (refs 3v4l.org/WR85Q).

The $return === null case is already covered in the code-paths, so the @-suppression operator could be introduced in front of the call to sscanf() and the error handler set-up (before) and tear-down (closer) be removed.

This would still need to pass the make, but this is how it could look like in (the relevant) code:

@@ -41,27 +41,19 @@
 
 	public function getScanfPlaceholdersCount(string $format): ?int
 	{
-		$throws = $this->phpVersion->throwsValueErrorForInternalFunctions();
-		try {
-			if ($throws === false) {
-				set_error_handler(
-					static function ($s, $m) {
-						throw new ValueError($m, 0);
-					},
-				);
-			}
-			$result = sscanf('', '%*n' . $format);
-			if ($result === null) {
-				return null;
-			}
-			return count($result);
-		} catch (ValueError) {
-			return null;
-		} finally {
-			if ($throws === false) {
-				restore_error_handler();
-			}
-		}
+		if ($this->phpVersion->throwsValueErrorForInternalFunctions()) {
+			try {
+				$result = sscanf('', '%*n' . $format);
+			} catch (ValueError) {
+				return null;
+			}
+		} else {
+			$result = @sscanf('', '%*n' . $format);
+		}
+		if ($result === null) {
+			return null;
+		}
+		return count($result);
 	}
 
 	/**

this is with an alignment to the other existing routine if(phpVersion->) direct call style.

It would spare the following uses:

• function restore_error_handler
• function set_error_handler

I'll push this once it passes. @staabm

Please find the code updated for the use of the @-suppression operator as well.

we have other places where we swallow runtime warnings with @ in case there is no other option. (there should be a test-case in the PR, which fails when the @ gets removed)

Oh I would love to add that, or this this with phpinfection covered in the CI? Just found the infection target, but it seems it's not part of the main goal?

How is this done with the @-suppression testing? Forgot to ask.

make sure we have a test which is run on CI PHP 7.x and uses a invalid pattern which would emit a warning on PHP 7.x.

PHPUnit will emit a new warning/error when the code under test emits warnings

Alternative suggestion: $resource -> STDIN in the file as in PHPStan/Rules/Functions/data/bug-10260.php

Please find it updated.

I would stay with $resource which the tests used before. pre-existing tests should not be changed whenever possible (only expectations should change). variable or constant might have different implications in static analysis context

I would stay with $resource which the tests used before. pre-existing tests should not be changed whenever possible (only expectations should change). variable or constant might have different implications in static analysis context

Yes, was also wondering about that so I tried it out ... please find the $str -> $resource by-catch fix updated, I missed your comment for the previous push so please excuse my ignorance re-requesting review a bit early.